mm: vmscan implement per-zone shrinkers

Hi,

I'm doing some works that require per-zone shrinkers, I'd like to get the vmscan part signed off and merged by interested mm people, please.

[And before anybody else kindly suggests per-node shrinkers, please go back and read all the discussion about this first.]

Quoted 5 lines · Nick Piggin

Hi,

I'm doing some works that require per-zone shrinkers, I'd like to get the vmscan part signed off and merged by interested mm people, please.

There are still plenty of unresolved issues with this general approach to scaling object caches that I'd like to see sorted out before we merge any significant shrinker API changes. Some things of

the top of my head:

- how to solve impendence mismatches between VM scalability techniques and subsystem scalabilty techniques that result in shrinker cross-muliplication explosions. e.g. XFS tracks reclaimable inodes in per-allocation group trees, so we'd get AG x per-zone LRU trees using this shrinker method. Think of the overhead on a 1000AG filesystem on a 1000 node machine with 3-5 zones per node....

- changes from global LRU behaviour to something that is not at all global - effect on workloads that depend on large scale caches that span multiple nodes is largely unknown. It will change IO patterns and affect system balance and performance of the system. How do we test/categorise/understand these problems and address such balance issues?

- your use of this shrinker architecture for VFS inode/dentry cache scalability requires adding lists and locks to the MM struct zone for each object cache type (inode, dentry, etc). As such, it is not a generic solution because it cannot be used for per-instance caches like the per-mount inode caches XFS uses.

i.e. nothing can actually use this infrastructure change without tying itself directly into the VM implementation, and even then not every existing shrinker can use this method of scaling. i.e. some level of abstraction from the VM implementation is needed in the shrinker API.

- it has been pointed out that slab caches are generally allocated out of a single zone per node, so per-zone shrinker granularity seems unnecessary.

- doesn't solve the unbound direct reclaim shrinker parallelism that is already causing excessive LRU lock contention on 8p single node systems. While per-LRU/per-node solves the larger scalability issue, it doesn't address scalability within the node. This is soon going to be 24p per node and that's more than enough to cause severe problems with a single lock and list...

Quoted 2 lines

[And before anybody else kindly suggests per-node shrinkers, please go back and read all the discussion about this first.]

I don't care for any particular solution, but I want these issues resolved before we make any move forward. per-node abstractions is just one possible way that has been suggested to address some of these issues, so it shouldn't be dismissed out of hand like this.

Quoted 7 lines · Dave Chinner

Quoted 5 lines · Nick Piggin

Hi,

I'm doing some works that require per-zone shrinkers, I'd like to get the vmscan part signed off and merged by interested mm people, please.

Quoted 3 lines

There are still plenty of unresolved issues with this general approach to scaling object caches that I'd like to see sorted out before we merge any significant shrinker API changes. Some things of

No changes, it just adds new APIs.

Quoted 3 lines

- it has been pointed out that slab caches are generally allocated out of a single zone per node, so per-zone shrinker granularity seems unnecessary.

No they are not, that's just total FUD. Where was that "pointed out"?

Slabs are generally allocated from every zone except for highmem and movable, and moreover there is nothing to prevent a shrinker implementation from needing to shrink highmem and movable pages as well.

Quoted 10 lines · Dave Chinner

Quoted 5 lines · Nick Piggin

Hi,

I'm doing some works that require per-zone shrinkers, I'd like to get the vmscan part signed off and merged by interested mm people, please.

There are still plenty of unresolved issues with this general approach to scaling object caches that I'd like to see sorted out before we merge any significant shrinker API changes. Some things of

No changes, it just adds new APIs.

Quoted 9 lines

the top of my head:

- how to solve impendence mismatches between VM scalability techniques and subsystem scalabilty techniques that result in shrinker cross-muliplication explosions. e.g. XFS tracks reclaimable inodes in per-allocation group trees, so we'd get AG x per-zone LRU trees using this shrinker method. Think of the overhead on a 1000AG filesystem on a 1000 node machine with 3-5 zones per node....

That's an interesting question, but no reason to hold anything up. It's up to the subsystem to handle it, or if they can't handle it sanely, you can elect to get no different behaviour than today.

I would say that per-zone is important for the above case, because it shrinks properly, based on memory pressure and pressure settings (eg. zone reclaim), and also obviously so that the reclaim threads are scanning node-local-memory so it doesn't have to pull objects from all over the interconnect.

With 1000 nodes and 1000AG filesystem, probably NxM is overkill, but there is no reason you couldn't have one LRU per X AGs.

All this is obviously implementation details -- pagecache reclaim does not want to know about this, so it doesn't affect the API.

Quoted 7 lines

- changes from global LRU behaviour to something that is not at all global - effect on workloads that depend on large scale caches that span multiple nodes is largely unknown. It will change IO patterns and affect system balance and performance of the system. How do we test/categorise/understand these problems and address such balance issues?

You've brought this up before several times and I've answered it.

The default configuration basically doesn't change much. Caches are allowed to fill up all zones and nodes, exactly the same as today.

On the reclaim side, when you have a global shortage, it will evenly shrink objects from all zones, so it still approximates LRU behaviour because number of objects far exceeds number of zones. This is exactly how per-zone pagecache scanning works too.

Quoted 6 lines

- your use of this shrinker architecture for VFS inode/dentry cache scalability requires adding lists and locks to the MM struct zone for each object cache type (inode, dentry, etc). As such, it is not a generic solution because it cannot be used for per-instance caches like the per-mount inode caches XFS uses.

Of course it doesn't. You can use kmalloc.

Quoted 5 lines

i.e. nothing can actually use this infrastructure change without tying itself directly into the VM implementation, and even then not every existing shrinker can use this method of scaling. i.e. some level of abstraction from the VM implementation is needed in the shrinker API.

"zones" is the abstraction. The thing which all allocation and management of memory is based on, everywhere you do any allocations in the kernel. A *shrinker* implementation, a thing which is called in response to memory shortage in a given zone, has to know about zones. End of story. "Tied to the memory management implementation" is true to the extent that it is part of the memory management implementation.

Quoted 3 lines

- it has been pointed out that slab caches are generally allocated out of a single zone per node, so per-zone shrinker granularity seems unnecessary.

No they are not, that's just total FUD. Where was that "pointed out"?

Slabs are generally allocated from every zone except for highmem and movable, and moreover there is nothing to prevent a shrinker implementation from needing to shrink highmem and movable pages as well.

Quoted 7 lines

- doesn't solve the unbound direct reclaim shrinker parallelism that is already causing excessive LRU lock contention on 8p single node systems. While per-LRU/per-node solves the larger scalability issue, it doesn't address scalability within the node. This is soon going to be 24p per node and that's more than enough to cause severe problems with a single lock and list...

It doesn't aim to solve that. It doesn't prevent that from being solved either (and improving parallelism in a subsystem generally, you know, helps with lock contention due to lots of threads in there, right?).

From that experience, what I suggest is that we provide the same

Quoted 25 lines · Nick Piggin

Quoted 10 lines · Dave Chinner

Quoted 5 lines · Nick Piggin

Hi,

I'm doing some works that require per-zone shrinkers, I'd like to get the vmscan part signed off and merged by interested mm people, please.

There are still plenty of unresolved issues with this general approach to scaling object caches that I'd like to see sorted out before we merge any significant shrinker API changes. Some things of

No changes, it just adds new APIs.

Quoted 9 lines

the top of my head:

- how to solve impendence mismatches between VM scalability techniques and subsystem scalabilty techniques that result in shrinker cross-muliplication explosions. e.g. XFS tracks reclaimable inodes in per-allocation group trees, so we'd get AG x per-zone LRU trees using this shrinker method. Think of the overhead on a 1000AG filesystem on a 1000 node machine with 3-5 zones per node....

Quoted 3 lines

That's an interesting question, but no reason to hold anything up. It's up to the subsystem to handle it, or if they can't handle it sanely, you can elect to get no different behaviour than today.

See, this is what I think is the problem - you answer any concerns by saying that the problem is entirely the responsibility of the subsystem to handle. You've decided how you want shrinkers to work, and everything else is SEP (somebody else's problem).

Quoted 8 lines

I would say that per-zone is important for the above case, because it shrinks properly, based on memory pressure and pressure settings (eg. zone reclaim), and also obviously so that the reclaim threads are scanning node-local-memory so it doesn't have to pull objects from all over the interconnect.

With 1000 nodes and 1000AG filesystem, probably NxM is overkill, but there is no reason you couldn't have one LRU per X AGs.

And when it's only 1 node or 50 nodes or 50 AGs or 5000 AGs? I don't like encoding _guesses_ at what might be a good configuration for an arbitrary NxM configuration because they are rarely right. That's exactly what you are suggesting here - that I solve this problem by making guesses at how it might scale and coming up with some arbitrary mapping scheme to handle it.

Quoted 2 lines

All this is obviously implementation details -- pagecache reclaim does not want to know about this, so it doesn't affect the API.

i.e. it's another SEP solution.

Quoted 10 lines

Quoted 7 lines

- changes from global LRU behaviour to something that is not at all global - effect on workloads that depend on large scale caches that span multiple nodes is largely unknown. It will change IO patterns and affect system balance and performance of the system. How do we test/categorise/understand these problems and address such balance issues?

You've brought this up before several times and I've answered it.

Quoted 7 lines

The default configuration basically doesn't change much. Caches are allowed to fill up all zones and nodes, exactly the same as today.

On the reclaim side, when you have a global shortage, it will evenly shrink objects from all zones, so it still approximates LRU behaviour because number of objects far exceeds number of zones. This is exactly how per-zone pagecache scanning works too.

The whole point of the zone-based reclaim is that shrinkers run when there are _local_ shortages on the _local_ LRUs and this will happen much more often than global shortages, especially on large machines. It will result in a change of behaviour, no question about it.

However, this is not reason for not moving to this model - what I'm asking is what the plan for categorising problems that arise as a result of such a change? How do we go about translating random reports like "I do this and it goes X% slower on 2.6.xx" to "that's a result of the new LRU reclaim model, and you should do <this> to try to resolve it". How do we triage such reports? What are our options to resolve such problems? Are there any knobs we should add at the beginning to give users ways of changing the behaviour to be more like the curent code? We've got lots of different knobs to control page cache reclaim behaviour - won't some of them be relevant to per-zone slab cache reclaim?

Quoted 8 lines

Quoted 6 lines

- your use of this shrinker architecture for VFS inode/dentry cache scalability requires adding lists and locks to the MM struct zone for each object cache type (inode, dentry, etc). As such, it is not a generic solution because it cannot be used for per-instance caches like the per-mount inode caches XFS uses.

Of course it doesn't. You can use kmalloc.

Right - another SEP solution.

The reason people are using shrinkers is that it is simple to implement a basic one. But implementing a scalable shrinker right now is fucking hard - look at all the problems we've had with XFS mostly because of the current API and the unbound parallelism of direct reclaim.

This new API is a whole new adventure that not many people are going to have machines capable of executing behavioural testing or stressing. If everyone is forced to implement their own "scalable shrinker" we'll end up with a rat's nest of different implementations with similar but subtly different sets of bugs...

Quoted 19 lines

Quoted 5 lines

i.e. nothing can actually use this infrastructure change without tying itself directly into the VM implementation, and even then not every existing shrinker can use this method of scaling. i.e. some level of abstraction from the VM implementation is needed in the shrinker API.

"zones" is the abstraction. The thing which all allocation and management of memory is based on, everywhere you do any allocations in the kernel. A *shrinker* implementation, a thing which is called in response to memory shortage in a given zone, has to know about zones. End of story. "Tied to the memory management implementation" is true to the extent that it is part of the memory management implementation.

Quoted 3 lines

- it has been pointed out that slab caches are generally allocated out of a single zone per node, so per-zone shrinker granularity seems unnecessary.

Quoted 1 line

No they are not, that's just total FUD. Where was that "pointed out"?

Christoph Hellwig mentioned it, I think Christoph Lameter asked exactly this question, I've mentioned it, and you even mentioned at one point that zones were effectively a per-node construct....

Quoted 13 lines

Slabs are generally allocated from every zone except for highmem and movable, and moreover there is nothing to prevent a shrinker implementation from needing to shrink highmem and movable pages as well.

Quoted 7 lines

- doesn't solve the unbound direct reclaim shrinker parallelism that is already causing excessive LRU lock contention on 8p single node systems. While per-LRU/per-node solves the larger scalability issue, it doesn't address scalability within the node. This is soon going to be 24p per node and that's more than enough to cause severe problems with a single lock and list...

Quoted 4 lines

It doesn't aim to solve that. It doesn't prevent that from being solved either (and improving parallelism in a subsystem generally, you know, helps with lock contention due to lots of threads in there, right?).

Sure, but you keep missing the point that it's a problem that is more important to solve for the typical 2S or 4S server than scaling

to 1000 nodes is. An architecture that allows unbounded parallelism is, IMO, fundamentally broken from a scalability point of view. Such behaviour indicates that the effects of such levels of parallelism weren't really considered with the subsystem was designed.

I make this point because of the observation I made that shrinkers are by far the most efficient when only a single thread is running reclaim on a particular LRU. Reclaim parallelism on a single LRU only made reclaim go slower and consume more CPU, and direct reclaim is definitely causing such parallelism and slowdowns (I can point you to the XFS mailing list posting again if you want).

I've previously stated that reducing/controlling the level of parallelism can be just as effective at providing serious scalability improvements as fine grained locking. So you don't simply scoff and mock me for suggesting it like you did last time,

Quoted 1 line

From that experience, what I suggest is that we provide the same

sort of _controlled parallelism_ for the generic shrinker infrastructure. We abstract out the lists and locks completely, and simply provide a "can you free this object" callback to the subsystem. I suggested we move shrinker callbacks out of direct reclaim to the (per-node) kswapd. having thought about it a bit more, a simpler (and probably better) alternative would be to allow only a single direct reclaimer to act on a specific LRU at a time, as this would allow different LRUs to be reclaimed in parallel. Either way, this bounds the amount of parallelism the shrinker will place on the lists and locks, thereby solving both the internal node scalability issues and the scaling to many nodes problems.

Such an abstraction also keeps all the details of how the objects are stored in LRUs and selected for reclaim completely hidden from the subsystems.

Quoted 4 lines · Dave Chinner

I've previously stated that reducing/controlling the level of parallelism can be just as effective at providing serious scalability improvements as fine grained locking. So you don't simply scoff and mock me for suggesting it like you did last time,

I didn't mock you. On the contrary I agreed that there are 2 problems here, and that lots of threads in reclaim is one of them. I know this

Hi,

I'm doing some works that require per-zone shrinkers, I'd like to get the vmscan part signed off and merged by interested mm people, please.

[And before anybody else kindly suggests per-node shrinkers, please go back and read all the discussion about this first.]

---
 fs/drop_caches.c    |    6 
 include/linux/mm.h  |   47 ++++++-
 mm/memory-failure.c |   10 -
 mm/vmscan.c         |  341 +++++++++++++++++++++++++++++++++++++---------------
 4 files changed, 297 insertions(+), 107 deletions(-)

Index: linux-2.6/include/linux/mm.h
===================================================================

include/linux/mm.h +18 -0

@@ -1008,6 +1008,10 @@ static inline void sync_mm_rss(struct ta
 /*
  * A callback you can register to apply pressure to ageable caches.
  *
+ * 'shrink_zone' is the new shrinker API. It is to be used in preference
+ * to 'shrink'. One must point to a shrinker function, the other must
+ * be NULL. See 'shrink_slab' for details about the shrink_zone API.
+ *
  * 'shrink' is passed a count 'nr_to_scan' and a 'gfpmask'.  It should
  * look through the least-recently-used 'nr_to_scan' entries and
  * attempt to free them up.  It should return the number of objects

@@ -1024,13 +1028,53 @@ struct shrinker {
 	int (*shrink)(struct shrinker *, int nr_to_scan, gfp_t gfp_mask);
 	int seeks;	/* seeks to recreate an obj */
 
+	/*
+	 * shrink_zone - slab shrinker callback for reclaimable objects
+	 * @shrink: this struct shrinker
+	 * @zone: zone to scan
+	 * @scanned: pagecache lru pages scanned in zone
+	 * @total: total pagecache lru pages in zone
+	 * @global: global pagecache lru pages (for zone-unaware shrinkers)
+	 * @flags: shrinker flags
+	 * @gfp_mask: gfp context we are operating within
+	 *
+	 * The shrinkers are responsible for calculating the appropriate
+	 * pressure to apply, batching up scanning (and cond_resched,
+	 * cond_resched_lock etc), and updating events counters including
+	 * count_vm_event(SLABS_SCANNED, nr).

include/linux/mm.h +17 -2

 	/* These are for internal use */
 	struct list_head list;
 	long nr;	/* objs pending delete */
 };
+
+/* Constants for use by old shrinker API */
 #define DEFAULT_SEEKS 2 /* A good number if you don't know better. */
+
+/* Constants for use by new shrinker API */
+/*
+ * SHRINK_DEFAULT_SEEKS is shifted by 4 to match an arbitrary constant
+ * in the old shrinker code.
+ */
+#define SHRINK_FACTOR	(128UL) /* Fixed point shift */
+#define SHRINK_DEFAULT_SEEKS	(SHRINK_FACTOR*DEFAULT_SEEKS/4)
+#define SHRINK_BATCH	128	/* A good number if you don't know better */
+
 extern void register_shrinker(struct shrinker *);
 extern void unregister_shrinker(struct shrinker *);
+extern void shrinker_add_scan(unsigned long *dst,
+				unsigned long scanned, unsigned long total,
+				unsigned long objects, unsigned int ratio);
+extern unsigned long shrinker_do_scan(unsigned long *dst, unsigned long batch);
 
 int vma_wants_writenotify(struct vm_area_struct *vma);
 

@@ -1464,8 +1508,7 @@ int in_gate_area_no_task(unsigned long a
 
 int drop_caches_sysctl_handler(struct ctl_table *, int,
 					void __user *, size_t *, loff_t *);
-unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask,
-			unsigned long lru_pages);
+void shrink_all_slab(struct zone *zone);
 
 #ifndef CONFIG_MMU
 #define randomize_va_space 0

Index: linux-2.6/mm/vmscan.c
===================================================================

mm/vmscan.c +26 -17

@@ -80,6 +80,9 @@ struct scan_control {
 	/* Can pages be swapped as part of reclaim? */
 	int may_swap;
 
+	/* Can slab pages be reclaimed? */
+	int may_reclaim_slab;
+
 	int swappiness;
 
 	int order;

@@ -169,6 +172,8 @@ static unsigned long zone_nr_lru_pages(s
  */
 void register_shrinker(struct shrinker *shrinker)
 {
+	BUG_ON(shrinker->shrink && shrinker->shrink_zone);
+	BUG_ON(!shrinker->shrink && !shrinker->shrink_zone);
 	shrinker->nr = 0;
 	down_write(&shrinker_rwsem);
 	list_add_tail(&shrinker->list, &shrinker_list);

@@ -187,43 +192,101 @@ void unregister_shrinker(struct shrinker
 }
 EXPORT_SYMBOL(unregister_shrinker);
 
-#define SHRINK_BATCH 128
 /*
- * Call the shrink functions to age shrinkable caches
+ * shrinker_add_scan - accumulate shrinker scan
+ * @dst: scan counter variable
+ * @scanned: pagecache pages scanned
+ * @total: total pagecache objects
+ * @tot: total objects in this cache
+ * @ratio: ratio of pagecache value to object value
  *
- * Here we assume it costs one seek to replace a lru page and that it also
- * takes a seek to recreate a cache object.  With this in mind we age equal
- * percentages of the lru and ageable caches.  This should balance the seeks
- * generated by these structures.
+ * shrinker_add_scan accumulates a number of objects to scan into @dst,
+ * based on the following ratio:
  *
- * If the vm encountered mapped pages on the LRU it increase the pressure on
- * slab to avoid swapping.
+ * proportion = scanned / total        // proportion of pagecache scanned
+ * obj_prop   = objects * proportion   // same proportion of objects
+ * to_scan    = obj_prop / ratio       // modify by ratio
+ * *dst += (total / scanned)           // accumulate to dst
  *
- * We do weird things to avoid (scanned*seeks*entries) overflowing 32 bits.
+ * The ratio is a fixed point integer with a factor SHRINK_FACTOR.
+ * Higher ratios give objects higher value.
  *
- * `lru_pages' represents the number of on-LRU pages in all the zones which
- * are eligible for the caller's allocation attempt.  It is used for balancing
- * slab reclaim versus page reclaim.
+ * @dst is also fixed point, so cannot be used as a simple count.
+ * shrinker_do_scan will take care of that for us.
  *
- * Returns the number of slab objects which we shrunk.
+ * There is no synchronisation here, which is fine really. A rare lost
+ * update is no huge deal in reclaim code.
  */
-unsigned long shrink_slab(unsigned long scanned, gfp_t gfp_mask,
-			unsigned long lru_pages)
+void shrinker_add_scan(unsigned long *dst,
+			unsigned long scanned, unsigned long total,
+			unsigned long objects, unsigned int ratio)
 {
-	struct shrinker *shrinker;
-	unsigned long ret = 0;

mm/vmscan.c +13 -0

+}
+EXPORT_SYMBOL(shrinker_add_scan);
+
+/*
+ * shrinker_do_scan - scan a batch of objects
+ * @dst: scan counter
+ * @batch: number of objects to scan in this batch
+ * @Returns: number of objects to scan
+ *
+ * shrinker_do_scan takes the scan counter accumulated by shrinker_add_scan,
+ * and decrements it by @batch if it is greater than batch and returns batch.
+ * Otherwise returns 0. The caller should use the return value as the number
+ * of objects to scan next.

mm/vmscan.c +50 -4

+}
+EXPORT_SYMBOL(shrinker_do_scan);
+
+#define SHRINK_BATCH 128
+/*
+ * Scan the deprecated shrinkers. This will go away soon in favour of
+ * converting everybody to new shrinker API.
+ */
+static void shrink_slab_old(unsigned long scanned, gfp_t gfp_mask,
+			unsigned long lru_pages)
+{
+	struct shrinker *shrinker;
 
 	list_for_each_entry(shrinker, &shrinker_list, list) {
 		unsigned long long delta;
 		unsigned long total_scan;
 		unsigned long max_pass;
 
+		if (!shrinker->shrink)
+			continue;
 		max_pass = (*shrinker->shrink)(shrinker, 0, gfp_mask);
 		delta = (4 * scanned) / shrinker->seeks;
 		delta *= max_pass;

@@ -250,15 +313,11 @@ unsigned long shrink_slab(unsigned long
 		while (total_scan >= SHRINK_BATCH) {
 			long this_scan = SHRINK_BATCH;
 			int shrink_ret;
-			int nr_before;
 
-			nr_before = (*shrinker->shrink)(shrinker, 0, gfp_mask);
 			shrink_ret = (*shrinker->shrink)(shrinker, this_scan,
 								gfp_mask);
 			if (shrink_ret == -1)
 				break;
-			if (shrink_ret < nr_before)
-				ret += nr_before - shrink_ret;
 			count_vm_events(SLABS_SCANNED, this_scan);
 			total_scan -= this_scan;
 

@@ -267,8 +326,86 @@ unsigned long shrink_slab(unsigned long
 
 		shrinker->nr += total_scan;
 	}
+}
+/*
+ * shrink_slab - Call the shrink functions to age shrinkable caches
+ * @zone: the zone we are currently reclaiming from
+ * @scanned: how many pagecache pages were scanned in this zone
+ * @total: total number of reclaimable pagecache pages in this zone
+ * @global: total number of reclaimable pagecache pages in the system
+ * @gfp_mask: gfp context that we are in
+ *
+ * Slab shrinkers should scan their objects in a proportion to the ratio of
+ * scanned to total pagecache pages in this zone, modified by a "cost"
+ * constant.
+ *
+ * For example, we have a slab cache with 100 reclaimable objects in a
+ * particular zone, and the cost of reclaiming an object is determined to be
+ * twice as expensive as reclaiming a pagecache page (due to likelihood and
+ * cost of reconstruction). If we have 200 reclaimable pagecache pages in that
+ * zone particular zone, and scan 20 of them (10%), we should scan 5% (5) of
+ * the objects in our slab cache.
+ *
+ * If we have a single global list of objects and no per-zone lists, the
+ * global count of objects can be used to find the correct ratio to scan.
+ *
+ * See shrinker_add_scan and shrinker_do_scan for helper functions and
+ * details on how to calculate these numbers.
+ */
+static void shrink_slab(struct zone *zone, unsigned long scanned,
+			unsigned long total, unsigned long global,
+			gfp_t gfp_mask)
+{
+	struct shrinker *shrinker;
+
+	if (scanned == 0)
+		scanned = SWAP_CLUSTER_MAX;
+
+	if (!down_read_trylock(&shrinker_rwsem))

mm/vmscan.c +40 -4

+	}
 	up_read(&shrinker_rwsem);
-	return ret;
+}
+
+/**
+ * shrink_all_slab - shrinks slabs in a given zone or system wide
+ * @zone: NULL to shrink slab from all zones, or non-NULL to shrink from a particular zone
+ *
+ * shrink_all_slab is a bit of a big hammer, and it's not really well defined what it should
+ * do (how much, how hard to shrink, etc), and it will throw out the reclaim balance. So it
+ * must only be used very carefully (drop_caches and hardware memory error handler are good
+ * examples).
+ */
+void shrink_all_slab(struct zone *zone)
+{
+	struct reclaim_state reclaim_state;
+
+	current->reclaim_state = &reclaim_state;
+	do {
+		reclaim_state.reclaimed_slab = 0;
+		/*
+		 * Use "100" for "scanned", "total", and "global", so
+		 * that shrinkers scan a large proportion of their
+		 * objects. 100 rather than 1 in order to reduce rounding
+		 * errors.
+		 */
+		if (!zone) {
+			for_each_populated_zone(zone)
+				shrink_slab(zone, 100, 100, 100, GFP_KERNEL);
+		} else
+			shrink_slab(zone, 100, 100, 100, GFP_KERNEL);
+	} while (reclaim_state.reclaimed_slab);
+
+	current->reclaim_state = NULL;
 }
 
 static void set_lumpy_reclaim_mode(int priority, struct scan_control *sc,

@@ -1801,16 +1938,22 @@ static void get_scan_count(struct zone *
  * This is a basic per-zone page freer.  Used by both kswapd and direct reclaim.
  */
 static void shrink_zone(int priority, struct zone *zone,
-				struct scan_control *sc)
+			struct scan_control *sc, unsigned long global_lru_pages)
 {
 	unsigned long nr[NR_LRU_LISTS];
 	unsigned long nr_to_scan;
 	enum lru_list l;
 	unsigned long nr_reclaimed = sc->nr_reclaimed;
 	unsigned long nr_to_reclaim = sc->nr_to_reclaim;
+	unsigned long nr_scanned = sc->nr_scanned;
+	unsigned long lru_pages = 0;
 
 	get_scan_count(zone, sc, nr, priority);
 
+	/* Used by slab shrinking, below */
+	if (sc->may_reclaim_slab)
+		lru_pages = zone_reclaimable_pages(zone);
+
 	while (nr[LRU_INACTIVE_ANON] || nr[LRU_ACTIVE_FILE] ||
 					nr[LRU_INACTIVE_FILE]) {
 		for_each_evictable_lru(l) {

@@ -1835,8 +1978,6 @@ static void shrink_zone(int priority, st
 			break;
 	}
 
-	sc->nr_reclaimed = nr_reclaimed;
-
 	/*
 	 * Even if we did not try to evict anon pages at all, we want to
 	 * rebalance the anon lru active/inactive ratio.

@@ -1844,6 +1985,23 @@ static void shrink_zone(int priority, st
 	if (inactive_anon_is_low(zone, sc))

mm/vmscan.c +13 -19

+			lru_pages, global_lru_pages, sc->gfp_mask);
+		if (reclaim_state) {
+			nr_reclaimed += reclaim_state->reclaimed_slab;
+			reclaim_state->reclaimed_slab = 0;
+		}
+	}
+
+	sc->nr_reclaimed = nr_reclaimed;
+
 	throttle_vm_writeout(sc->gfp_mask);
 }
 

@@ -1864,7 +2022,7 @@ static void shrink_zone(int priority, st
  * scan then give up on it.
  */
 static void shrink_zones(int priority, struct zonelist *zonelist,
-					struct scan_control *sc)
+		struct scan_control *sc, unsigned long global_lru_pages)
 {
 	struct zoneref *z;
 	struct zone *zone;

@@ -1884,7 +2042,7 @@ static void shrink_zones(int priority, s
 				continue;	/* Let kswapd poll it */
 		}
 
-		shrink_zone(priority, zone, sc);
+		shrink_zone(priority, zone, sc, global_lru_pages);
 	}
 }
 

@@ -1941,7 +2099,6 @@ static unsigned long do_try_to_free_page
 {
 	int priority;
 	unsigned long total_scanned = 0;
-	struct reclaim_state *reclaim_state = current->reclaim_state;
 	struct zoneref *z;
 	struct zone *zone;
 	unsigned long writeback_threshold;

@@ -1953,30 +2110,20 @@ static unsigned long do_try_to_free_page
 		count_vm_event(ALLOCSTALL);
 
 	for (priority = DEF_PRIORITY; priority >= 0; priority--) {
-		sc->nr_scanned = 0;
-		if (!priority)
-			disable_swap_token();
-		shrink_zones(priority, zonelist, sc);
-		/*
-		 * Don't shrink slabs when reclaiming memory from
-		 * over limit cgroups
-		 */
-		if (scanning_global_lru(sc)) {
-			unsigned long lru_pages = 0;
-			for_each_zone_zonelist(zone, z, zonelist,
-					gfp_zone(sc->gfp_mask)) {
-				if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
-					continue;
+		unsigned long lru_pages = 0;
 
-				lru_pages += zone_reclaimable_pages(zone);
-			}
+		for_each_zone_zonelist(zone, z, zonelist,

mm/vmscan.c +28 -34

 		}
+
+		sc->nr_scanned = 0;
+		if (!priority)
+			disable_swap_token();
+		shrink_zones(priority, zonelist, sc, lru_pages);
 		total_scanned += sc->nr_scanned;
 		if (sc->nr_reclaimed >= sc->nr_to_reclaim)
 			goto out;

@@ -2029,6 +2176,7 @@ unsigned long try_to_free_pages(struct z
 		.nr_to_reclaim = SWAP_CLUSTER_MAX,
 		.may_unmap = 1,
 		.may_swap = 1,
+		.may_reclaim_slab = 1,
 		.swappiness = vm_swappiness,
 		.order = order,
 		.mem_cgroup = NULL,

@@ -2058,6 +2206,7 @@ unsigned long mem_cgroup_shrink_node_zon
 		.may_writepage = !laptop_mode,
 		.may_unmap = 1,
 		.may_swap = !noswap,
+		.may_reclaim_slab = 0,
 		.swappiness = swappiness,
 		.order = 0,
 		.mem_cgroup = mem,

@@ -2076,7 +2225,7 @@ unsigned long mem_cgroup_shrink_node_zon
 	 * will pick up pages from other mem cgroup's as well. We hack
 	 * the priority and make it zero.
 	 */
-	shrink_zone(0, zone, &sc);
+	shrink_zone(0, zone, &sc, zone_reclaimable_pages(zone));
 
 	trace_mm_vmscan_memcg_softlimit_reclaim_end(sc.nr_reclaimed);
 

@@ -2094,6 +2243,7 @@ unsigned long try_to_free_mem_cgroup_pag
 		.may_writepage = !laptop_mode,
 		.may_unmap = 1,
 		.may_swap = !noswap,
+		.may_reclaim_slab = 0,
 		.nr_to_reclaim = SWAP_CLUSTER_MAX,
 		.swappiness = swappiness,
 		.order = 0,

@@ -2171,11 +2321,11 @@ static unsigned long balance_pgdat(pg_da
 	int priority;
 	int i;
 	unsigned long total_scanned;
-	struct reclaim_state *reclaim_state = current->reclaim_state;
 	struct scan_control sc = {
 		.gfp_mask = GFP_KERNEL,
 		.may_unmap = 1,
 		.may_swap = 1,
+		.may_reclaim_slab = 1,
 		/*
 		 * kswapd doesn't want to be bailed out while reclaim. because
 		 * we want to put equal scanning pressure on each zone.

@@ -2249,7 +2399,6 @@ static unsigned long balance_pgdat(pg_da
 		 */
 		for (i = 0; i <= end_zone; i++) {
 			struct zone *zone = pgdat->node_zones + i;
-			int nr_slab;
 
 			if (!populated_zone(zone))
 				continue;

@@ -2271,15 +2420,11 @@ static unsigned long balance_pgdat(pg_da
 			 */
 			if (!zone_watermark_ok(zone, order,
 					8*high_wmark_pages(zone), end_zone, 0))
-				shrink_zone(priority, zone, &sc);
-			reclaim_state->reclaimed_slab = 0;
-			nr_slab = shrink_slab(sc.nr_scanned, GFP_KERNEL,
-						lru_pages);
-			sc.nr_reclaimed += reclaim_state->reclaimed_slab;
+				shrink_zone(priority, zone, &sc, lru_pages);
 			total_scanned += sc.nr_scanned;
 			if (zone->all_unreclaimable)
 				continue;
-			if (nr_slab == 0 && !zone_reclaimable(zone))
+			if (!zone_reclaimable(zone))
 				zone->all_unreclaimable = 1;
 			/*
 			 * If we've done a decent amount of scanning and

@@ -2545,6 +2690,7 @@ unsigned long shrink_all_memory(unsigned
 		.may_swap = 1,
 		.may_unmap = 1,
 		.may_writepage = 1,
+		.may_reclaim_slab = 1,
 		.nr_to_reclaim = nr_to_reclaim,
 		.hibernation_mode = 1,
 		.swappiness = vm_swappiness,

@@ -2728,13 +2874,14 @@ static int __zone_reclaim(struct zone *z
 		.may_writepage = !!(zone_reclaim_mode & RECLAIM_WRITE),
 		.may_unmap = !!(zone_reclaim_mode & RECLAIM_SWAP),
 		.may_swap = 1,
+		.may_reclaim_slab = 0,
 		.nr_to_reclaim = max_t(unsigned long, nr_pages,
 				       SWAP_CLUSTER_MAX),
 		.gfp_mask = gfp_mask,
 		.swappiness = vm_swappiness,
 		.order = order,
 	};
-	unsigned long nr_slab_pages0, nr_slab_pages1;
+	unsigned long lru_pages, slab_pages;
 
 	cond_resched();
 	/*

@@ -2747,51 +2894,61 @@ static int __zone_reclaim(struct zone *z
 	reclaim_state.reclaimed_slab = 0;
 	p->reclaim_state = &reclaim_state;
 
+	lru_pages = zone_reclaimable_pages(zone);
+	slab_pages = zone_page_state(zone, NR_SLAB_RECLAIMABLE);
+
 	if (zone_pagecache_reclaimable(zone) > zone->min_unmapped_pages) {
+		if (slab_pages > zone->min_slab_pages)
+			sc.may_reclaim_slab = 1;
 		/*
 		 * Free memory by calling shrink zone with increasing
 		 * priorities until we have enough memory freed.
 		 */
 		priority = ZONE_RECLAIM_PRIORITY;
 		do {
-			shrink_zone(priority, zone, &sc);
+			shrink_zone(priority, zone, &sc, lru_pages);
 			priority--;
 		} while (priority >= 0 && sc.nr_reclaimed < nr_pages);
-	}
 
-	nr_slab_pages0 = zone_page_state(zone, NR_SLAB_RECLAIMABLE);
-	if (nr_slab_pages0 > zone->min_slab_pages) {
+	} else if (slab_pages > zone->min_slab_pages) {
 		/*
-		 * shrink_slab() does not currently allow us to determine how
-		 * many pages were freed in this zone. So we take the current
-		 * number of slab pages and shake the slab until it is reduced
-		 * by the same nr_pages that we used for reclaiming unmapped
-		 * pages.
-		 *
-		 * Note that shrink_slab will free memory on all zones and may
-		 * take a long time.
+		 * Scanning slab without pagecache, have to open code
+		 * call to shrink_slab (shirnk_zone drives slab reclaim via
+		 * pagecache scanning, so it isn't set up to shrink slab
+		 * without scanning pagecache.
 		 */
-		for (;;) {
-			unsigned long lru_pages = zone_reclaimable_pages(zone);
-
-			/* No reclaimable slab or very low memory pressure */
-			if (!shrink_slab(sc.nr_scanned, gfp_mask, lru_pages))
-				break;
 
-			/* Freed enough memory */
-			nr_slab_pages1 = zone_page_state(zone,
-							NR_SLAB_RECLAIMABLE);
-			if (nr_slab_pages1 + nr_pages <= nr_slab_pages0)
-				break;
-		}
+		/*
+		 * lru_pages / 10  -- put a 10% pressure on the slab

mm/vmscan.c +16 -5

 		/*
-		 * Update nr_reclaimed by the number of slab pages we
-		 * reclaimed from this zone.
+		 * Although we have a zone based slab shrinker API, some slabs
+		 * are still scanned globally. This means we can't quite
+		 * determine how many pages were freed in this zone by
+		 * checking reclaimed_slab. However the regular shrink_zone
+		 * paths have exactly the same problem that they largely
+		 * ignore. So don't be different.
+		 *
+		 * The situation will improve dramatically as important slabs
+		 * are switched over to using reclaimed_slab after the
+		 * important slabs are converted to using per zone shrinkers.
+		 *
+		 * Note that shrink_slab may free memory on all zones and may
+		 * take a long time, but again switching important slabs to
+		 * zone based shrinkers will solve this problem.
 		 */
-		nr_slab_pages1 = zone_page_state(zone, NR_SLAB_RECLAIMABLE);
-		if (nr_slab_pages1 < nr_slab_pages0)
-			sc.nr_reclaimed += nr_slab_pages0 - nr_slab_pages1;
+		sc.nr_reclaimed += reclaim_state.reclaimed_slab;
+		reclaim_state.reclaimed_slab = 0;
 	}
 
 	p->reclaim_state = NULL;

Index: linux-2.6/fs/drop_caches.c
===================================================================

fs/drop_caches.c +1 -5

@@ -35,11 +35,7 @@ static void drop_pagecache_sb(struct sup
 
 static void drop_slab(void)
 {
-	int nr_objects;
-
-	do {
-		nr_objects = shrink_slab(1000, GFP_KERNEL, 1000);
-	} while (nr_objects > 10);
+	shrink_all_slab(NULL); /* NULL - all zones */
 }
 
 int drop_caches_sysctl_handler(ctl_table *table, int write,

Index: linux-2.6/mm/memory-failure.c
===================================================================

mm/memory-failure.c +2 -9

@@ -235,14 +235,8 @@ void shake_page(struct page *p, int acce
 	 * Only all shrink_slab here (which would also
 	 * shrink other caches) if access is not potentially fatal.
 	 */
-	if (access) {
-		int nr;
-		do {
-			nr = shrink_slab(1000, GFP_KERNEL, 1000);
-			if (page_count(p) == 1)
-				break;
-		} while (nr > 10);
-	}
+	if (access)
+		shrink_all_slab(page_zone(p));
 }
 EXPORT_SYMBOL_GPL(shake_page);
 
 
--

To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Quoted 13 lines

mm/vmscan.c +0 -2

@@ -1835,8 +1978,6 @@ static void shrink_zone(int priority, st
 			break;
 	}
 
-	sc->nr_reclaimed = nr_reclaimed;
-
 	/*
 	 * Even if we did not try to evict anon pages at all, we want to
 	 * rebalance the anon lru active/inactive ratio.

@@ -1844,6 +1985,23 @@ static void shrink_zone(int priority, st
 	if (inactive_anon_is_low(zone, sc))
 		shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
 

mm/vmscan.c static void shrink_zone(int priority, st · line 1995

Quoted 8 lines

mm/vmscan.c +8 -0

+	/*
+	 * Don't shrink slabs when reclaiming memory from
+	 * over limit cgroups
+	 */
+	if (sc->may_reclaim_slab) {
+		struct reclaim_state *reclaim_state = current->reclaim_state;
+
+		shrink_slab(zone, sc->nr_scanned - nr_scanned,

Doubtful calculation. What mean "sc->nr_scanned - nr_scanned"? I think nr_scanned simply keep old slab balancing behavior.

mm/vmscan.c static void shrink_zone(int priority, st · line 2006

Quoted 13 lines

mm/vmscan.c +9 -0

+			lru_pages, global_lru_pages, sc->gfp_mask);
+		if (reclaim_state) {
+			nr_reclaimed += reclaim_state->reclaimed_slab;
+			reclaim_state->reclaimed_slab = 0;
+		}
+	}
+
+	sc->nr_reclaimed = nr_reclaimed;
+
 	throttle_vm_writeout(sc->gfp_mask);
 }

Quoted 13 lines

Quoted 13 lines

mm/vmscan.c +0 -2

@@ -1835,8 +1978,6 @@ static void shrink_zone(int priority, st
 			break;
 	}
 
-	sc->nr_reclaimed = nr_reclaimed;
-
 	/*
 	 * Even if we did not try to evict anon pages at all, we want to
 	 * rebalance the anon lru active/inactive ratio.

@@ -1844,6 +1985,23 @@ static void shrink_zone(int priority, st
 	if (inactive_anon_is_low(zone, sc))
 		shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
 

Quoted 11 lines

mm/vmscan.c static void shrink_zone(int priority, st · line 1995

Quoted 8 lines

mm/vmscan.c +8 -0

+	/*
+	 * Don't shrink slabs when reclaiming memory from
+	 * over limit cgroups
+	 */
+	if (sc->may_reclaim_slab) {
+		struct reclaim_state *reclaim_state = current->reclaim_state;
+
+		shrink_slab(zone, sc->nr_scanned - nr_scanned,

Doubtful calculation. What mean "sc->nr_scanned - nr_scanned"? I think nr_scanned simply keep old slab balancing behavior.

And per-zone reclaim can lead to new issue. On 32bit highmem system, theorically the system has following memory usage.

ZONE_HIGHMEM: 100% used for page cache
ZONE_NORMAL:  100% used for slab

So, traditional page-cache/slab balancing may not work. I think following

new calculation or somethinhg else is necessary.

	if (zone_reclaimable_pages() > NR_SLAB_RECLAIMABLE) {
		using current calculation
	} else {
		shrink number of "objects >> reclaim-priority" objects
		(as page cache scanning calculation)
	}

However, it can be separate this patch, perhaps.

Nick, I want to test your tree.
This is taking too long.
Make something available now. And test it in real configs.

Quoted 5 lines · Anca Emanuel

Nick, I want to test your tree.
This is taking too long.
Make something available now. And test it in real configs.

hi Anca,

would you like to give your test method?

Quoted 7 lines · Figo.zhang

ao? 11/16/2010 03:53 PM, Anca Emanuel a??e??:

Quoted 5 lines

Nick, I want to test your tree.
This is taking too long.
Make something available now. And test it in real configs.

Quoted 4 lines

hi Anca,

would you like to give your test method?

Nothig special, for now I will test on my PC.

Quoted 11 lines · Anca Emanuel

Quoted 8 lines · Figo.zhang

Quoted 5 lines · Anca Emanuel

Nick, I want to test your tree.
This is taking too long.
Make something available now. And test it in real configs.

Quoted 7 lines

Quoted 4 lines

hi Anca,

would you like to give your test method?

Nothig special, for now I will test on my PC.

hi KOSAKI Motohiro,

is it any test suite or test scripts for test page-reclaim performance?

Best,
Figo.zhang

Quoted 22 lines · Figo.zhang

Quoted 11 lines · Anca Emanuel

Quoted 8 lines · Figo.zhang

Quoted 5 lines · Anca Emanuel

Nick, I want to test your tree.
This is taking too long.
Make something available now. And test it in real configs.

Quoted 7 lines

Quoted 4 lines

hi Anca,

would you like to give your test method?

Nothig special, for now I will test on my PC.

hi KOSAKI Motohiro,

Quoted 5 lines

is it any test suite or test scripts for test page-reclaim performance?

Best,
Figo.zhang

There is http://www.phoronix.com

Quoted 26 lines · Anca Emanuel

Quoted 24 lines · Figo.zhang

Quoted 11 lines · Anca Emanuel

Quoted 8 lines · Figo.zhang

Quoted 5 lines · Anca Emanuel

Nick, I want to test your tree.
This is taking too long.
Make something available now. And test it in real configs.

Quoted 7 lines

Quoted 4 lines

hi Anca,

would you like to give your test method?

Nothig special, for now I will test on my PC.

hi KOSAKI Motohiro,

is it any test suite or test scripts for test page-reclaim performance?

Quoted 5 lines

Quoted 3 lines

Best,
Figo.zhang

There is http://www.phoronix.com

it is not focus on page-reclaim test, or specially for MM.

Quoted 1 line

Quoted 11 lines

Quoted 10 lines

Quoted 8 lines

hi KOSAKI Motohiro,

is it any test suite or test scripts for test page-reclaim performance?

Best,
Figo.zhang

There is http://www.phoronix.com

Quoted 4 lines

it is not focus on page-reclaim test, or specially for MM.

Quoted 1 line

If you want some special test, you have to ask Michael Larabel for that.
http://www.phoronix-test-suite.com/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email:<a href=ilto:"[email protected]"> [email protected]</a>

Quoted 16 lines

Quoted 15 lines

Quoted 10 lines

Quoted 8 lines

hi KOSAKI Motohiro,

is it any test suite or test scripts for test page-reclaim performance?

Best,
Figo.zhang

There is http://www.phoronix.com

it is not focus on page-reclaim test, or specially for MM.

Quoted 1 line

Quoted 2 lines

If you want some special test, you have to ask Michael Larabel for that.
http://www.phoronix-test-suite.com/

yes, i see, the phoronix-test-suite is test such as ffmpeg, games. not focus on MM.

Nick, I want to test your tree.
This is taking too long.
Make something available now. And test it in real configs.

Quoted 28 lines

Quoted 13 lines

Quoted 13 lines

mm/vmscan.c +0 -2

@@ -1835,8 +1978,6 @@ static void shrink_zone(int priority, st
 			break;
 	}
 
-	sc->nr_reclaimed = nr_reclaimed;
-
 	/*
 	 * Even if we did not try to evict anon pages at all, we want to
 	 * rebalance the anon lru active/inactive ratio.

@@ -1844,6 +1985,23 @@ static void shrink_zone(int priority, st
 	if (inactive_anon_is_low(zone, sc))
 		shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
 

Quoted 11 lines

mm/vmscan.c static void shrink_zone(int priority, st · line 1995

Quoted 8 lines

mm/vmscan.c +8 -0

+	/*
+	 * Don't shrink slabs when reclaiming memory from
+	 * over limit cgroups
+	 */
+	if (sc->may_reclaim_slab) {
+		struct reclaim_state *reclaim_state = current->reclaim_state;
+
+		shrink_slab(zone, sc->nr_scanned - nr_scanned,

Doubtful calculation. What mean "sc->nr_scanned - nr_scanned"? I think nr_scanned simply keep old slab balancing behavior.

And per-zone reclaim can lead to new issue. On 32bit highmem system, theorically the system has following memory usage.

Quoted 4 lines

ZONE_HIGHMEM: 100% used for page cache
ZONE_NORMAL:  100% used for slab

So, traditional page-cache/slab balancing may not work. I think following

Yes, in theory you are right. I guess in theory the same hole exists if we have 0% page cache reclaimable globally, but this may be slightly more likely to hit.

Quoted 9 lines

new calculation or somethinhg else is necessary.

	if (zone_reclaimable_pages() > NR_SLAB_RECLAIMABLE) {
		using current calculation
	} else {
		shrink number of "objects >> reclaim-priority" objects
		(as page cache scanning calculation)
	}

Quoted 1 line

However, it can be separate this patch, perhaps.

I agree. In fact, perhaps the new calculation would work well in all cases anyway, so maybe we should move away from making slab reclaim a slave to pagecache reclaim.

Can we approach that in subsequent patches?

Hi

Quoted 5 lines

Hi,

I'm doing some works that require per-zone shrinkers, I'd like to get the vmscan part signed off and merged by interested mm people, please.

Quoted 2 lines

[And before anybody else kindly suggests per-node shrinkers, please go back and read all the discussion about this first.]

vmscan part looks good to me. however I hope fs folks review too even though I'm not sure who is best.

btw, I have some nitpick comments. see below.

include/linux/mm.h struct shrinker { · line 1054

Quoted 5 lines

include/linux/mm.h +5 -0

+	void (*shrink_zone)(struct shrinker *shrink,
+		struct zone *zone, unsigned long scanned,
+		unsigned long total, unsigned long global,
+		unsigned long flags, gfp_t gfp_mask);
+

shrink_zone is slightly grep unfriendly. Can you consider shrink_slab_zone() or something else?

Quoted 6 lines

mm/vmscan.c +3 -2

+void shrinker_add_scan(unsigned long *dst,
+			unsigned long scanned, unsigned long total,
+			unsigned long objects, unsigned int ratio)
 {
-	struct shrinker *shrinker;
-	unsigned long ret = 0;

mm/vmscan.c void unregister_shrinker(struct shrinker

Quoted 10 lines

mm/vmscan.c +6 -2

+	unsigned long long delta;
 
-	if (scanned == 0)
-		scanned = SWAP_CLUSTER_MAX;
+	delta = (unsigned long long)scanned * objects;
+	delta *= SHRINK_FACTOR;
+	do_div(delta, total + 1);
 
+	delta *= SHRINK_FACTOR; /* ratio is also in SHRINK_FACTOR units */
+	do_div(delta, ratio + 1);

introdusing tiny macro is better than the comment.

Quoted 1 line

mm/vmscan.c void unregister_shrinker(struct shrinker · line 240

Quoted 11 lines

mm/vmscan.c +9 -2

-	if (!down_read_trylock(&shrinker_rwsem))
-		return 1;	/* Assume we'll be able to shrink next time */
+	/*
+	 * Avoid risking looping forever due to too large nr value:
+	 * never try to free more than twice the estimate number of
+	 * freeable entries.
+	 */
+	*dst += delta;
+
+	if (*dst / SHRINK_FACTOR > objects)
+		*dst = objects * SHRINK_FACTOR;

objects * SHRINK_FACTOR appear twice in this function. calculate "objects = obj * SHRINK_FACTOR" at first improve code readability slightly.

Quoted 1 line

+unsigned long shrinker_do_scan(unsigned long *dst, unsigned long batch)

Seems misleading name a bit. shrinker_do_scan() does NOT scan. It only does batch adjustment.

mm/vmscan.c void unregister_shrinker(struct shrinker · line 265

Quoted 2 lines

mm/vmscan.c +2 -0

+{
+	unsigned long nr = ACCESS_ONCE(*dst);

Dumb question: why is this ACCESS_ONCE() necessary?

mm/vmscan.c void unregister_shrinker(struct shrinker · line 269

Quoted 4 lines

mm/vmscan.c +4 -0

+	if (nr < batch * SHRINK_FACTOR)
+		return 0;
+	*dst = nr - batch * SHRINK_FACTOR;
+	return batch;

{
	unsigned long nr = ACCESS_ONCE(*dst);
	batch *= SHRINK_FACTOR;

	if (nr < batch)
		return 0;
	*dst = nr - batch;
	return batch;
}

is slighly cleaner. however It's unclear why dst and batch argument need to have different unit (i.e why caller can't do batch * FACTOR?).

mm/vmscan.c unsigned long shrink_slab(unsigned long · line 374

Quoted 5 lines

mm/vmscan.c +5 -0

+	list_for_each_entry(shrinker, &shrinker_list, list) {
+		if (!shrinker->shrink_zone)
+			continue;
+		(*shrinker->shrink_zone)(shrinker, zone, scanned,
+					total, global, 0, gfp_mask);

flags argument is unused?

Quoted 4 lines

mm/vmscan.c +0 -0

@@ -1844,6 +1985,23 @@ static void shrink_zone(int priority, st
 	if (inactive_anon_is_low(zone, sc))
 		shrink_active_list(SWAP_CLUSTER_MAX, zone, sc, priority, 0);
 

mm/vmscan.c static void shrink_zone(int priority, st · line 1995

Quoted 8 lines

mm/vmscan.c +8 -0

+	/*
+	 * Don't shrink slabs when reclaiming memory from
+	 * over limit cgroups
+	 */
+	if (sc->may_reclaim_slab) {
+		struct reclaim_state *reclaim_state = current->reclaim_state;
+
+		shrink_slab(zone, sc->nr_scanned - nr_scanned,

Doubtful calculation. What mean "sc->nr_scanned - nr_scanned"? I think nr_scanned simply keep old slab balancing behavior.

Quoted 5 lines

mm/vmscan.c +4 -0

+		for_each_zone_zonelist(zone, z, zonelist,
+				gfp_zone(sc->gfp_mask)) {
+			if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
+				continue;
 

mm/vmscan.c static unsigned long do_try_to_free_page · line 2120

Quoted 6 lines

mm/vmscan.c +1 -5

-			shrink_slab(sc->nr_scanned, sc->gfp_mask, lru_pages);
-			if (reclaim_state) {
-				sc->nr_reclaimed += reclaim_state->reclaimed_slab;
-				reclaim_state->reclaimed_slab = 0;
-			}
+			lru_pages += zone_reclaimable_pages(zone);

Do we really need this doubtful cpuset hardwall filtering? Why do we need to change slab reclaim pressure if cpuset is used. In old days, we didn't have per-zone slab shrinker, then we need artificial slab pressure boost for preventing false positive oom-killer. but now we have.

However, If you strongly keep old behavior at this time, I don't oppose. We can change it later.

Quoted 6 lines

mm/vmscan.c +6 -0

+		/*
+		 * lru_pages / 10  -- put a 10% pressure on the slab
+		 * which roughly corresponds to ZONE_RECLAIM_PRIORITY
+		 * scanning 1/16th of pagecache.
+		 *
+		 * Global slabs will be shrink at a relatively more

mm/vmscan.c static int __zone_reclaim(struct zone *z · line 2932

Quoted 6 lines

mm/vmscan.c +6 -0

+		 * aggressive rate because we don't calculate the
+		 * global lru size for speed. But they really should
+		 * be converted to per zone slabs if they are important
+		 */
+		shrink_slab(zone, lru_pages / 10, lru_pages, lru_pages,
+				gfp_mask);

Why don't you use sc.nr_scanned? It seems straight forward.